Remove incorrect handling of covariant return types #83
Conversation
|
I also propose cutting 1.16.1 with this fix because many projects out there are using 1.16 |
|
I have been bitten by this several times (#77). Does removing the check allow animal-sniffer to catch the problem? |
Correct. Covariant return types are purely a compile-time construct, so this check is extraneous. |
|
@ogolberg No it's not just a compiletime construct -- your code will compile fine with a covariant change in return type (in particular if you ignore the return value), but then can fail at runtime with an exception. |
|
@lukehutch yes, I think we're on the same page here. What I mean is that the compiler does all the work to handle covariant return types (https://docs.oracle.com/javase/tutorial/java/generics/bridgeMethods.html#bridgeMethods), there's nothing special in the bytecode/vm to make them work; therefore, the special casing that I am removing in this PR is unnecessary. |
|
But Animal Sniffer should fail and stop compilation if a covariant return type is detected. The current code detects the change but doesn't stop the build with an error. How does removing the check altogether address this problem? |
|
the |
|
But then there will be no descriptive error message given. See my PR #87 for an alternative. (I'm not sure if the logic is correct though.) |
|
sure, additional logging may be worth the cost of running that |
|
any chance this can be merged? |
|
why removing an integration test? |
|
restored/updated integration tests |
Addresses incorrect handling of covariant return types (i.e. the infamous ByteBuffer API issue) when validating android compatibility (see mojohaus/animal-sniffer#83).
This removes incorrect covariant return type handling introduced in MANIMALSNIFFER-49. The ticket is no longer publicly accessible, so I lack the context of the original change.
The block of code removed in this PR prevented animalsniffer from catching covariant changes to return types which were compile-time compatible but not binary-compatible.
For example, if we take the code below
and compile it on Java 8, we'll get bytecode which invokes
If we compile it on Java 9+, while still targeting Java 8, we will get bytecode which invokes
That's because this method with the more specific
ByteBufferreturn type is available in JDK9+.Now, if we run the latter binary on Java 8, we will get a
NoSuchMethodError.A recent example of this in the wild: grpc/grpc-java#6839 (note that grpc-java uses animalnsniffer).